-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29586 Setting hbase.oldwals.cleaner.thread.size to negative value will throw IllegalArgumentExce… #7293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses HBASE-29586 by adding validation to prevent IllegalArgumentException when the hbase.oldwals.cleaner.thread.size configuration is set to a negative or zero value.
- Adds input validation to check for non-positive thread pool size values
- Logs a warning and uses default value when invalid configuration is detected
- Prevents runtime exceptions by early return when invalid values are encountered
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
谢谢,邮件已收到,祝生活愉快,工作顺利!
|
| LOG.warn( | ||
| "The configuration {} has been set to an invalid value {}, " | ||
| + "the default value {} will be used, no need to update.", | ||
| OLD_WALS_CLEANER_THREAD_SIZE, newSize, DEFAULT_OLD_WALS_CLEANER_THREAD_SIZE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, wait a minute, here we just return, which means we will keep the old value, not use the default value?
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
谢谢,邮件已收到,祝生活愉快,工作顺利!
|
|
Thanks for the fix! |
| public void onConfigurationChange(Configuration conf) { | ||
| int newSize = conf.getInt(OLD_WALS_CLEANER_THREAD_SIZE, DEFAULT_OLD_WALS_CLEANER_THREAD_SIZE); | ||
| if (newSize <= 0) { | ||
| LOG.warn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe just put as debug. also maybe sync with impl at
hbase/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCleanerChore.java
Line 180 in 8b17149
| if (newThreadCount < 1) { |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
…ue will throw IllegalArgumentException (#7293) Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Signed-off-by: Peng Lu <[email protected]>
…ue will throw IllegalArgumentException (#7293) Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Signed-off-by: Peng Lu <[email protected]>
…ue will throw IllegalArgumentException (#7293) Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Signed-off-by: Peng Lu <[email protected]>
…ue will throw IllegalArgumentException (apache#7293) Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Signed-off-by: Peng Lu <[email protected]>
jira:HBASE-29586